Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: DEBUG harmony #8136

Merged
merged 7 commits into from
Feb 6, 2024
Merged

fix: DEBUG harmony #8136

merged 7 commits into from
Feb 6, 2024

Conversation

erights
Copy link
Member

@erights erights commented Aug 2, 2023 •
edited
Loading

closes: #8096

Updates agoric-sdk to use the new conveniences exported by @endo/env-options.

Reform the contents of the DEBUG environment variable, i.e., the process.env.DEBUG option to be a better behaved list for agoric:* options. See the new docs/env.md for details.

NOTE: This reform of the meaning of the agoric:* members of the DEBUG list is in theory a compat issue. But because we have no known occurrences of that, and it would only affect debugging and logging behavior anyway, we do not flag this PR as a breaking change.

@erights erights self-assigned this Aug 2, 2023
@erights erights force-pushed the markm-8096-DEBUG-harmony branch from 59fcb74 to f3c70b4 Compare August 4, 2023 21:25
@erights erights mentioned this pull request Aug 4, 2023
Closed
michaelfig
michaelfig reviewed Aug 7, 2023
View reviewed changes
docs/env.md Outdated Show resolved Hide resolved
@erights erights force-pushed the markm-8096-DEBUG-harmony branch 3 times, most recently from 1284f64 to 641d38e Compare August 11, 2023 04:48
@erights erights force-pushed the markm-8096-DEBUG-harmony branch from 641d38e to 03d9e36 Compare August 22, 2023 02:41
@erights erights changed the base branch from master to markm-8231-interfaces-be-passable August 22, 2023 02:42
@erights erights changed the base branch from markm-8231-interfaces-be-passable to master August 28, 2023 05:57
@erights erights force-pushed the markm-8096-DEBUG-harmony branch from 03d9e36 to efe7dce Compare August 28, 2023 05:58
@erights
Copy link
Member Author

erights commented Aug 29, 2023

See also nodejs/node#48890

@erights erights force-pushed the markm-8096-DEBUG-harmony branch 2 times, most recently from ff5ba9e to 9443819 Compare September 16, 2023 03:45
@erights erights force-pushed the markm-8096-DEBUG-harmony branch from 9443819 to 8b88047 Compare October 14, 2023 01:52
@erights erights force-pushed the markm-8096-DEBUG-harmony branch from 8b88047 to ef3b2a4 Compare October 24, 2023 23:39
@erights erights force-pushed the markm-8096-DEBUG-harmony branch from ef3b2a4 to c4116e9 Compare November 7, 2023 20:06
@erights erights force-pushed the markm-8096-DEBUG-harmony branch from c4116e9 to 03999ed Compare December 21, 2023 04:02
@erights erights mentioned this pull request Jan 1, 2024
Merged
@erights erights force-pushed the markm-8096-DEBUG-harmony branch from 03999ed to a023b50 Compare January 1, 2024 00:23
@erights erights mentioned this pull request Jan 1, 2024
Merged
@michaelfig michaelfig force-pushed the markm-8096-DEBUG-harmony branch 3 times, most recently from 7456ba3 to f0f4290 Compare January 7, 2024 01:59
@michaelfig
Copy link
Member

nvm my earlier (deleted) comment about the CI failure. It seems a perfect specimen of the #endo-branch: failures we were seeing on other PRs, so I will try to fix it in this PR.

@michaelfig michaelfig force-pushed the markm-8096-DEBUG-harmony branch 5 times, most recently from 94c6cbf to 531eb51 Compare January 7, 2024 04:17
@erights erights force-pushed the markm-8096-DEBUG-harmony branch from 992e080 to b715c58 Compare January 29, 2024 08:23
@erights erights changed the base branch from mfig-build-uses-hashes to mark-rebaseof-mfig-build-uses-hashes January 29, 2024 08:24
@erights erights mentioned this pull request Jan 29, 2024
Closed
@erights erights force-pushed the markm-8096-DEBUG-harmony branch from ea1aa1c to 2a47191 Compare January 30, 2024 22:05
@erights erights changed the base branch from mark-rebaseof-mfig-build-uses-hashes to master January 30, 2024 22:05
@erights erights force-pushed the markm-8096-DEBUG-harmony branch 2 times, most recently from fa82f4b to 9a47f38 Compare February 3, 2024 04:53
@erights erights changed the title WIP fix: DEBUG harmony fix: DEBUG harmony Feb 3, 2024
@erights
Copy link
Member Author

erights commented Feb 3, 2024

Hi @michaelfig , this is Ready for Review.

@erights erights marked this pull request as ready for review February 3, 2024 05:00
@erights erights requested a review from michaelfig February 3, 2024 05:00
@erights
Copy link
Member Author

erights commented Feb 3, 2024

@michaelfig , could you take a look at the failing integration tests? Thanks.

@michaelfig
Copy link
Member

could you take a look at the failing integration tests?

This has been bumped up in my queue. I ground to a halt the last time I tried; with fresh eyes I should get it done this week.

@michaelfig
Copy link
Member

@erights, I pushed some changes to restore some of the logging default behaviour. This was to cause silencing (using the info level as the minimum default) only if $DEBUG was explicitly set to the empty string. Not setting $DEBUG at all, or setting it to anything non-empty, would use the log level as the minimum default.

Let's see this pass CI, then I will review again.

@michaelfig michaelfig force-pushed the markm-8096-DEBUG-harmony branch from 9a47f38 to 3a6cd01 Compare February 5, 2024 17:34
@erights
Copy link
Member Author

erights commented Feb 5, 2024

Let's see this pass CI, then I will review again.

It passed, thanks much!

@erights erights force-pushed the markm-8096-DEBUG-harmony branch 2 times, most recently from 7de259b to afcb935 Compare February 5, 2024 22:31
@erights erights force-pushed the markm-8096-DEBUG-harmony branch from afcb935 to 6d75b5b Compare February 5, 2024 22:48
michaelfig
michaelfig approved these changes Feb 6, 2024
View reviewed changes
Copy link
Member

@michaelfig michaelfig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please address comments, but otherwise this LGTM!

docs/env.md Outdated
@@ -97,6 +102,10 @@ Description: When nonempty, create pretend prepopulated tokens like "moola" and

Lifetime: until chain is mature enough not to need any pretend tokens

## LOCKDOWN_*

For the envoronment variables beginning with `LOCKDOWN_` , see [`lockdown` Options](https://github.com/endojs/endo/blob/master/packages/ses/docs/lockdown.md).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
For the envoronment variables beginning with `LOCKDOWN_` , see [`lockdown` Options](https://github.com/endojs/endo/blob/master/packages/ses/docs/lockdown.md).
For the environment variables beginning with `LOCKDOWN_` , see [`lockdown` Options](https://github.com/endojs/endo/blob/master/packages/ses/docs/lockdown.md).

packages/solo/src/start.js Outdated
Comment on lines 379 to 382
} else if (DEBUG_LIST.includes('agoric:info')) {
verbosity = ['-v'];
} else {
verbosity = [];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I overlooked changing this. Please update:

Suggested change
} else if (DEBUG_LIST.includes('agoric:info')) {
verbosity = ['-v'];
} else {
verbosity = [];
} else if (DEBUG_LIST.includes('agoric:info') || process.env.DEBUG='') {
verbosity = [];
} else {
verbosity = ['-v'];

@erights erights added the automerge:squash Automatically squash merge label Feb 6, 2024
@mergify mergify bot merged commit d2ea4b4 into master Feb 6, 2024
70 of 77 checks passed
@mergify mergify bot deleted the markm-8096-DEBUG-harmony branch February 6, 2024 05:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michaelfig michaelfig michaelfig approved these changes

Assignees

@erights erights

Labels
automerge:squash Automatically squash merge force:integration Force integration tests to run on PR
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Inconsistency between endo vs agoric-sdk on syntax of DEBUG options
2 participants
@erights @michaelfig